Transforms haml forms to react for Add and Edit feature of Automate Class #9301
Conversation
elsamaryv
changed the title
elsamaryv
changed the title
elsamaryv
force-pushed
the
class-form-converison
branch
from
2dcfb86 to
7572497
Compare
|
@miq-bot add_label enhancement |
|
@miq-bot assign @GilbertCherrie |
elsamaryv
force-pushed
the
class-form-converison
branch
2 times, most recently
from
0493f7b to
73c00d5
Compare
elsamaryv
force-pushed
the
class-form-converison
branch
from
f9920d1 to
d359d80
Compare
| request | ||
| .then((response) => { | ||
| if (response.status === 200) { | ||
| const confirmation = isEdit ? __(`Class "${values.name}" was saved`) : __(`Class "${values.name}" was added`); |
There was a problem hiding this comment.
For translations in the UI code, you have to use placeholders within the translated text and then fill them in afterwards with something like sprintf. See https://github.com/ManageIQ/guides/blob/master/i18n.md#javascript, particularly the caveats section. I think this might work:
| const confirmation = isEdit ? __(`Class "${values.name}" was saved`) : __(`Class "${values.name}" was added`); | |
| let confirmation = isEdit ? __(`Class "%s" was saved`) : __(`Class "%s" was added`); | |
| confirmation = sprintf(confirmation, value.name); |
There was a problem hiding this comment.
Thanks for pointing that out. I've updated that part.
| }; | ||
|
|
||
| const onCancel = () => { | ||
| const confirmation = classRecord.id ? __(`Edit of Class "${classRecord.name}" cancelled by the user`) |
cypress.config.js
Outdated
| @@ -7,7 +7,7 @@ module.exports = defineConfig({ | |||
| baseUrl: 'http://localhost:3000', | |||
| viewportHeight: 800, | |||
| viewportWidth: 1800, | |||
| numTestsKeptInMemory: 0, | |||
| numTestsKeptInMemory: 5, | |||
There was a problem hiding this comment.
Sorry, I changed this while debugging and forgot to set it back to 0 when pushing. We can put it back to 0.
|
Overall LGTM. I kind of don't like the way the Fully Qualified Name is presented...it gets kind of hidden. Is it possible to use a textbox around it that is readonly (but still allow copy)? Additionally, and this is admittedly a change in functionality, but can we change the Fully Qualified string presentation to not have spaces around the |
I've added text field around the Fully Qualified Name. Got rid of the spaces and the text can be copied as well. May I know your thoughts on this? (Please see the screenshots) @Fryguy |
|
Looks much better - do you know if that read-only field is still copy-able? |
| import miqFlash from '../../helpers/miq-flash'; | ||
|
|
||
| const MiqAeClass = ({ classRecord, fqname }) => { | ||
| const formattedFqname = fqname.replace(/\s+/g, ''); |
There was a problem hiding this comment.
Is this implying that the fqname comes back from the API with spaces?
I was actually expecting any client-side formatting code to just be deleted, so this is surprising.
EDIT: Here's what the backend returns, which is why I'm surprised:
irb(main):001> MiqAeClass.first.fqname
=> "/ManageIQ/AutomationManagement/AnsibleTower/Operations/JobTemplate"
There was a problem hiding this comment.
I was going through the MiqAeClassController and saw this line of code which is creating those spaces.
Not sure if this line was added intentionally. If it's removed, it may affect the Fully Qualified Name displayed during the creation/editing of Domain, Namespace, Class (and potentially other areas). Could you please confirm if it's fine to remove it? @Fryguy
Yes, it can be selected and copied. |
Automation / Automate / Explorer / Datastore
Transforms haml forms to react for Add and Edit feature of Automate Class.
Before
After